Skip to content

Bound credential operations and prevent authentication resource contention - #2082

Draft
Tyrie Vella (tyrielv) wants to merge 2 commits into
microsoft:vnextfrom
tyrielv:tyrielv/auth-lock-contention
Draft

Tyrie Vella (tyrielv) wants to merge 2 commits into
microsoft:vnextfrom
tyrielv:tyrielv/auth-lock-contention

Conversation

@tyrielv

@tyrielv Tyrie Vella (tyrielv) commented Aug 11, 2026 •

Copy link
Copy Markdown
Contributor

Problem and Context

Runtime credential helpers can wait indefinitely during object downloads and background prefetch.

A missed Git Credential Manager prompt can block maintenance while it holds the shared prefetch lock.

Credential work can also hold shared HTTP capacity. Concurrent authentication failures can then block healthy hydration requests.

Changes

  • Apply a configurable deadline to runtime credential fill, approve, reject, and reload operations.
  • Treat values of zero or less as the escape hatch for the historical unbounded wait.
  • Serialize all credential-helper verbs and prevent competing helpers.
  • Cancel initialization, authentication-lock, and credential-gate waits before helper ownership.
  • After ownership, use the configured deadline instead of request cancellation.
  • Use typed timeout state instead of credential-helper stderr text.
  • Stop retries, endpoint fallback, and outer hydration retries after a terminal credential timeout.
  • Bound process exit and output drain waits, and terminate the process tree after timeout or incomplete drain.
  • Release the HTTP connection permit before credential rejection.
  • Prevent duplicate permit releases after cancellation or another exception.
  • Emit structured timeout telemetry without credential-helper stderr or secret material.
  • Add unit tests for timeout configuration, deadline sharing, retry suppression, contention, cancellation, process waits, and permit accounting.

Known gaps and deliberate decisions

  • Process boundary coverage — real-process tests cover timeout and cancellation. Partial tree failure and inherited-handle drain do not have deterministic test fixtures.
  • Telemetry taxonomy — timeout events identify fill, reload, delete, and approve failures. Gate contention and backoff suppression do not have separate event types.

@tyrielv
Tyrie Vella (tyrielv) force-pushed the tyrielv/auth-lock-contention branch from c2b26d1 to 840201a Compare August 18, 2026 20:51
@tyrielv
Tyrie Vella (tyrielv) changed the base branch from master to vnext August 18, 2026 20:52
@tyrielv Tyrie Vella (tyrielv) changed the title Auth concurrency: release HTTP pool slot before credential reject; thread cancellation to the credential path (stacked on #2046) Bound credential operations and prevent authentication resource contention Sep 22, 2026
@tyrielv
Tyrie Vella (tyrielv) force-pushed the tyrielv/auth-lock-contention branch 2 times, most recently from 7dc9cf0 to e65cc6e Compare September 29, 2026 22:56
The runtime credential path (HttpRequestor.SendRequest ->
GitAuthentication.TryGetCredentials/RejectCredentials ->
TryCallGitCredential) called git-credential with timeoutMs = -1, so
Process.WaitForExit(-1) waited forever. When a GCM auth popup was missed
(e.g. behind another window), the mount's background maintenance
PrefetchStep blocked indefinitely while holding the shared
prefetch-commits-trees.lock, which in turn blocked a user-initiated
`gvfs prefetch`.

The mount startup auth path was already bounded via credentialTimeoutMs;
this extends the same bound to every runtime credential invocation:

- TryGetCredentials takes credentialTimeoutMs (default
  DefaultCredentialTimeoutMs) and plumbs it to TryCallGitCredential.
- RejectCredentials, which reloads the credential on the 401-retry leg,
  takes and plumbs the same timeout (this leg is the actual stale-token
  hang path and was otherwise still unbounded).
- ApproveCredentials, RejectCredentials and the ICredentialStore
  store/delete operations are bounded too. `git credential approve` and
  `git credential reject` previously ran with timeoutMs = -1 while
  holding gitAuthLock, so a stalled helper could still pin the prefetch
  lock forever even after the fill leg was bounded.
- HttpRequestor exposes a protected virtual CredentialTimeoutMs and
  passes it to TryGetCredentials, RejectCredentials and
  ApproveCredentials.

The bound is generous (120s) rather than the 30s default: the mount's
requestor is shared by the background maintenance prefetch, interactive
on-demand hydration, and the user-initiated prefetch/clone verbs, where a
human may legitimately take longer than 30s to answer a GCM cold-start /
MFA / smartcard prompt. 120s still bounds the hang while being long
enough not to cut off a prompt the user is actively answering.

The value lives on RetryConfig, which is already loaded once from git
config and already passed into the HttpRequestor constructor alongside
MaxRetries and Timeout. It is overridable via
gvfs.credential-timeout-seconds; 0 or less restores the old unbounded
wait as a field escape hatch. Loading it through RetryConfig keeps timeout parsing with the existing
retry settings and avoids a config read for each request.

The credential serialization gate now waits at least as long as the fetch
it is serializing. It previously waited a fixed 60s, and on expiry fell
through and spawned a second credential fetch. With a 120s fetch bound
that guaranteed a second, competing GCM prompt in exactly the slow-prompt
case the longer bound exists to tolerate.

A timed-out fetch no longer asks the caller to retry. SendRequest
previously returned shouldRetry: true for every credential failure, so a
timeout burned the whole RetryWrapper budget (up to MaxAttempts x 120s),
re-prompting the user each time. TryGetCredentials now reports whether
the failure was a timeout, and SendRequest sets shouldRetry accordingly;
genuine auth failures still retry as before.

On timeout the git process tree is killed, not just git.exe. Killing only
git.exe left the credential helper child alive, holding the credential
store and showing orphaned prompt UI. The kill is now followed by a
bounded wait so the async stdout/stderr readers flush before their
buffers are read.

The timeout is reported as a distinct CredentialFetchTimedOut telemetry
event with structured timeoutMs and RepoAuthority fields, rather than only as
warning message text. This is what makes the 120s choice measurable in
the field: how often the bound fires, and whether a timeout is followed
by a successful fetch (a prompt that was cut off) or not (a hang that was
prevented).

On timeout the fetch fails, backoff engages, the download gives up, and
the lock is released instead of hanging forever.

Tests: MockGitProcess now records the timeout passed to each git
invocation, so tests can assert the bound is actually plumbed rather than
just that a failure message appears. The timeout test asserts the
observed timeout and the rendered "within 1 seconds" message; reverting
the plumbing makes it fail (verified by mutation). Adds a test that the
401-reject leg bounds both the credential reload and the erase, a test
that only genuine timeouts are reported as timeouts so a real auth
failure still retries (also mutation-verified), and RetryConfig coverage
for the default, configured, and unbounded-escape-hatch values.

Assisted-by: Claude Opus 4.8
Signed-off-by: Tyrie Vella <tyrielv@gmail.com>
@tyrielv
Tyrie Vella (tyrielv) force-pushed the tyrielv/auth-lock-contention branch 2 times, most recently from 9dcaa53 to 61933b0 Compare November 4, 2026 19:31
…tions

Serialize credential helpers and preserve terminal timeout state across retries and endpoint fallback.

Make waits cancelable before helper ownership. After ownership, use the configured deadline instead of request cancellation.

Release HTTP capacity before credential rejection and prevent duplicate permit releases.

Use typed timeout results. Bound process exit and output drain waits, and contain partial process-tree termination failures.

Add regression tests for timeout, contention, cancellation, process cleanup, and retry suppression.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@tyrielv
Tyrie Vella (tyrielv) force-pushed the tyrielv/auth-lock-contention branch from 61933b0 to 08ad141 Compare November 4, 2026 22:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant